This should not happen in ping - it's not used in this function and get_docs depends on it. You should initialise it in such a way, that both ping and get_docs could work if called individually, not sequentially.

Understood your concern here. But, the connector is designed in such a way that ping will always gets executed first and then the get_docs method. So, there would not be a chance of the get_docs method to be called individually since the framework itself is not allowing it. However, once we have a conclusion on this comment, we'll update the code accordingly.

It's not designed this way, it works this way now because it was the most convenient way to do. It might and will change in future, so we want to build connectors that are resilient to changes.

In short, you should treat ping and get_docs() as methods that can be called in any sequence OR even on their own, all these 3 cases are valid:

1. ping(); get_docs()
2. ping(); ping()
3. get_docs()

Oh! got it. However this is related to this comment so do you want me to change this before getting the conclusion on it, or should wait for it? Till the time, i'll address other comments.

I'll try to address the topic in the comment in the mean time, I'll try to get back to you asap

Sure, please. Do let me know your opinion on this.

Keeping this to both the places i.e. ping and get_docs

Ideally you should test multiple cases with different data types returned here, it's your main function that you implement and it has only single test for a very basic scenario, can you make sure you test the method properly?

It would be helpful if you provide some more idea which kind of test you want for this method? Since this test is covering multiple data types including project, issue and attachment.

Will this still work if self.is_cloud is True?

Yes, this will work for both cloud and on-prem.

I just want to confirm, if self.is_cloud is True, you are trying to use service_account_id and api_token to build a basic auth.

Is this line needed? If you exit the infinite loop above, does it mean both project_task and attachment_task are done?

Yes, this line runs the producers. Once, the await is finished, both the project_task and attachment_task are completed. The consumer loop is independent from this gather. The responsibility of this loop is to consume the documents added by the producer tasks. This consumer loop will only end once it gets the required number of FINISHED in the queue which are eventually added by the producer tasks themselves. Hence, in a way, consumer and producer are communicating via FINISHED signal.

I would really like to see the same pattern we've used for the Confluence connector (see: https://github.com/elastic/connectors-python/pull/568/files), i.e. creating a JiraClient class, which encapsulates the interaction with the server/cloud instance. Makes the connectors way more consistent.

Sure, however the testing cycle has been completed and it would take quite some efforts from the QA end to re-test it with new changes, so I strongly recommend we raise a GitHub issue and cover it as an enhancement PR. Let me know your thoughts regarding the same.

pep8 like others, so :

/rest/api/2/search?maxResults={max_results}&startAt={start_at}

Got it

if it's MiB, add 1024*1024 here

see my previous comment about MiB

pep8

logger.warning ?

Yes, we can put a warning logger instead.

cc @artem-shelkovnikov this is where #534 would be useful

let's try except this with just a warning if remove failed

what does pyright says here?

Error we're getting here -

nice!

if total does not exists you get None and the next test will fail.

if you always get total and it's an int, change this to:

total = response_json["total"]

if not, make that code more robust:

total = int(response_json.get("total", 0))

please add an issue about this so we have the same behavior for convert_to_b64 on all platform

why do you feed when ? it looks like the default value as it's done in iso_utc

Since, we're indexing project every time with _timestamp with iso_utc and other jira documents are using the timestamp getting from the response. So, if Jira server is hosted in different timezone other than UTC, user will face timestamp mismatch for projects and other objects in term of timezone. So, keep it consitent, we're feeding when to the iso_utc of the timezone of Jira server.

why copy here?

Here, attachment is a dictionary so any changes in the partial should not be reflected in the below code. Hence, we passed the copy to avoid changes done by the partial in the passed param.

nice!